fix(frontend): redirect to default environment when URL env id is ialid#7734
Open
SketchRudy wants to merge 1 commit into
Open
fix(frontend): redirect to default environment when URL env id is ialid#7734SketchRudy wants to merge 1 commit into
SketchRudy wants to merge 1 commit into
Conversation
…alid Closes Flagsmith#7446 When a URL contains an environment identifier that does not correspond to a real environment for the current project (e.g. /project/1/environment/undefined/features), the dashboard previously showed an infinite loading spinner. The root cause: 1. useFeatureListWithApiKey returns skipToken when the env api_key does not resolve, so the features query is permanently skipped and isLoading never flips to false. 2. EnvironmentReadyChecker silently ignored the 404 from useGetEnvironmentQuery and fell through to render children with the invalid environmentId. EnvironmentReadyChecker now redirects to /project/:projectId/ when either the loaded environments list does not contain the URL's environmentId, or the per-environment query returns 404. That route already handles "pick the default environment" via ProjectRedirectPage, so no new default-environment logic is introduced. The redirect decision is extracted into a pure helper (common/utils/shouldRedirectMissingEnvironment) with unit tests covering nine cases including the ref guard against double-fire, partial loading states, zero-environment projects, and non-404 errors. Also switches the checker from useRouteMatch to useRouteContext to match the convention used by FeaturesPage and ProjectRedirectPage, and removes the now-unused match prop passed in by ParameterizedRoute. Acknowledges the reverted PR Flagsmith#7284 history: that attempt rendered a NotFoundState UX which was reverted by Flagsmith#7312; this PR takes the redirect approach explicitly called out in Flagsmith#7446's acceptance criterion.
|
@SketchRudy is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
docs/if required so people know about the feature.Changes
Closes #7446
When a URL contains an environment identifier that does not correspond to a real environment for the current project (e.g.
/project/1/environment/undefined/features), the dashboard previously showed an infinite loading spinner. Root cause:useFeatureListWithApiKeyreturnsskipTokenwhen the env api_key does not resolve, so the features query is permanently skipped andisLoadingnever flips tofalse.EnvironmentReadyCheckersilently ignored the 404 fromuseGetEnvironmentQueryand fell through to render its children with the invalidenvironmentId.EnvironmentReadyCheckernow redirects to/project/:projectId/when either the loaded environments list does not contain the URL'senvironmentId, or the per-environment query returns a 404. That route already handles "pick the default environment" viaProjectRedirectPage, so no new default-environment logic is introduced.The redirect decision is extracted into a pure helper (
common/utils/shouldRedirectMissingEnvironment) with unit tests covering nine cases including the ref guard against double-fire, partial loading states, zero-environment projects, and non-404 errors.Notes for review
NotFoundStateUX which was reverted by fix: revert prevent infinite API loop on invalid environment URLs #7312. This PR takes the redirect approach explicitly called out in Features list loops in loading forever #7446's acceptance criterion.useRouteMatchtouseRouteContextto matchFeaturesPage/ProjectRedirectPage, and the now-unusedmatchprop fromParameterizedRouteis removed.How did you test this code?
Automated
bash cd frontend npx jest common/utils/__tests__/shouldRedirectMissingEnvironment.test.ts Expected:
9 passed. Type-check and lint are clean on the changed files.Manual reproduction
With a local stack (
ENV=local npm run devagainst an API on localhost:8000), logged into a project with at least one environment:/project/1/environment/undefined/features- before: infinite spinner + repeatingenvironments/undefined/404s; after: URL flips to the default environment, list renders./project/1/environment/create- env-create flow loads (checker skipped)./project/99999/environment/undefined/features- bounces through/project/99999/and inherits ProjectRedirectPage's org-home fallback.